Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Create OnOffToggle component for preferences. #2323

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Create OnOffToggle component for preferences. #2323

wants to merge 5 commits into from

Conversation

lindapaiste
Copy link
Collaborator

@lindapaiste lindapaiste commented Jul 24, 2023 •
edited
Loading

This is purely a code cleanup, there is no change to UI or behavior. It extracts a lot of repetitive code in the Preferences component into a new component OnOffToggle.

The component is inspired by @ghalestrilo's optionsOnOff code for the mobile preferences (which will be going away, in favor of a single responsive component).

It will probably evolve more as I refactor other parts of the Preferences.

Changes:

  • Create a component OnOffToggle for the clickable "On" and "Off" labels.
    image
  • Write unit tests.
  • Use the OnOffToggle in Preferences for Autosave, Autoclose Brackets and Quotes, etc.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste
Copy link
Collaborator Author

lindapaiste commented Jul 24, 2023 •
edited
Loading

Maybe I should rework the PreferencePicker instead of making a whole new component? 🤷

PiyushChandra17
PiyushChandra17 approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@PiyushChandra17 PiyushChandra17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely Done

@lindapaiste
Merge remote-tracking branch 'upstream/develop' into refactor/prefs-o…
60063ae 
…noff

# Conflicts:
#	client/modules/IDE/components/Preferences/index.jsx
@lindapaiste
Copy link
Collaborator Author

Possibly translationKey="LineNumbers" is a bit too "tricky" and it would be be better to be more explicit, even though it's more verbose. I can change the syntax to ariaLabelOn={t('Preferences.LineNumbersOnARIA')} ariaLabelOff={t('Preferences.LineNumbersOffARIA')}.

@lindapaiste lindapaiste mentioned this pull request Nov 28, 2023
Open
@raclim raclim added this to the MINOR Release for 2.13.0 milestone Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PiyushChandra17 PiyushChandra17 PiyushChandra17 approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
MINOR Release for 2.13.0
Development

Successfully merging this pull request may close these issues.
3 participants
@lindapaiste @PiyushChandra17 @raclim